Repository navigation
Conversation
| // consumes that macro call from the token stream, so tok always advances. | ||
| /** Expand while the expansion result in @p temp ends with the name of a | ||
| * function-like macro whose arguments are supplied by the tokens after @p tok. | ||
| * Each round consumes that macro call from the token stream, so tok always |
There was a problem hiding this comment.
I am not sure that "so tok always advances" will be true now. If rescanMacro returns false directly should we return tok->next?
There was a problem hiding this comment.
I updated it to return tok->next instead relying on the callers to call tok->next.
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
I found no new problems. When a macro argument expands to the name of a function-like macro and the call's ( ... ) comes later in the same argument, the PR now expands that call. I tested about 50 variations, and every one that changed now matches gcc and clang. Every remaining difference from them also exists on base. I couldn't find an input that this PR handles wrongly. The only test added (define27) is a fuzzer-style case whose expected output is empty. It doesn't show the basic behaviour, such as ID(G()(1)) → [1], so a few direct regression tests would help.
Testing
Build and test suites
make -j8 testrunner simplecpp CXXOPTS=-Werrorinpr/built with no warnings../testrunnerpassed.python3 run-tests.py: 0 failed, 16 todos.c99-6_10_3_4_p5.cnow passes.python3 -m pytest integration_test.py: 128 passed../selfcheck.shpassed.- I also built
base/to compare results.
Comparison with gcc and clang
I wrote three files in scratch/ (t1.c, t2.c, t3.c) and compared pr/simplecpp, base/simplecpp, gcc -E -P and clang -E -P after removing spaces. They share these definitions:
#define f(x) <x> /* [x] in t1.c and t2.c */
#define ID(x) x
#define G() fThe cases covered:
- Calls inside the argument:
ID(G()(1)),ID(G()(G()(2))),f(G()(f(3))),ID(G()((a,b))). - The macro name as the trailing token:
#define GI() IDwithID(GI()(1)), andID(ID(I)(0)(6)). - Calls that alternate or refer to themselves:
#define A(x) B/#define B(x) AwithID(A(1)(2)(3)(4)), and#define R() RwithID(R()()()). - Nested and repeated arguments: two calls inside a two-parameter macro's arguments, the argument used twice (
W(G()(4))), an object-like macro (ID(OBJ(5))), andGG()()(10). - Variadic macros,
#xthroughXNUM, and##. - A call that is completed only after the argument ends:
ID(G())(1). - The
(on the next line. - An error case:
ID(G()(1,2))gives "Wrong number of parameters for macro 'f'". gcc and clang also report an error.
In all of these the PR output matches gcc and clang. The cases that base got wrong are now right, for example:
ID(A(1)(2)(3)(4)): base gaveB(2)(3)(4), the PR givesA.ID(G()(f)(f)(3)): base gavef(f)(f)(3), the PR gives[f](f)(3).
Two cases still differ from gcc, and base gives the same output for both, so the PR did not cause them:
ID(G() E (8))with an emptyE: simplecpp givesf(8), gcc gives<8>.C2(G(), (3))with#define C2(a,b) a b: simplecpp givesf(3), gcc gives[3].
Performance
A file with 60000 lines of nested B(B(n)) macros, each argument expanded more than once, took about 3.6 s with the PR and 3.7 s with base, so there is no slowdown.
Automated review of 148b1c2 by Claude. The pull request was built and tested locally.
| "#define k() l(j(, ), )\n" | ||
| "#define n(a) j(l(o, )(a), )\n" | ||
| "n(k)\n"; | ||
| ASSERT_EQUALS("", preprocess(code)); |
There was a problem hiding this comment.
AI review
The only new test is a large, fuzzer-style case whose expected output is "", so it doesn't show what behaviour it protects. Please add a few direct cases, for example with #define f(x) [x], #define ID(x) x and #define G() f:
ID(G()(1))→[ 1 ]ID(G()(G()(2)))→[ [ 2 ] ]#define A(x) B/#define B(x) AwithID(A(1)(2)(3)(4))→A(each round must use the right set of already-expanded macros)
I checked all three against gcc, and they pass with this PR and fail on base.
Fixes #549